fix(builder): validation of template accesses which depend on chainId#1691
fix(builder): validation of template accesses which depend on chainId#1691
chainId#1691Conversation
or other globals this basically turned into a refactor because we have to kind of rethink how we are supplying values to the ChainDefinition. But I think its a move in the right direction. Unfortunately refactors like this usually lead to breakage, so probably best to save this for next release.
| new ChainDefinition(deployInfo.def, false, { | ||
| chainId, | ||
| timestamp: deployInfo.timestamp || 0, | ||
| package: { version: '0.0.0' }, | ||
| }), |
There was a problem hiding this comment.
Having to create this dummy object on several places just to be able to instantiate looks prone to error, maybe a helper function to create the ChainDefinition would be useful?
There was a problem hiding this comment.
Oh, I see that those are overrides, but are they necessary? looks like they are only used for giving them to the access recorder, in that case wouldn't be enough to just send the dummy object? Why is there an option to send custom overrides?
| name = "greeter" | ||
| version = "<%= package.version %>" | ||
| description = "Simple project to verify the functionality of cannon" | ||
| keywords = ["sample", "greeter"] |
There was a problem hiding this comment.
Would be good to add an action on this or other of the cannonfiles used on the test with Inifinex's case, was is with something like <%= settings.smth['VALUE_' + chainId] %>?
|
Something appears to be fishy? socket reports a lot of dependencies changes here: #1691 (comment), but there are no changes on any |
or other globals
this basically turned into a refactor because we have to kind of rethink how we are supplying values to the ChainDefinition. But I think its a move in the right direction.
Unfortunately refactors like this usually lead to breakage, so probably best to save this for next release.